get contact pictures from social networks #1580

call-me-matt · 2020-04-14T18:38:17Z

Fix #1546
Implements partly #20

README.md

templates/avatars.php

src/components/Settings/SettingsAddressbook.vue

src/components/ContactDetails/ContactDetailsAvatar.vue

lib/Controller/AvatarController.php

skjnldsv · 2020-04-15T06:35:16Z

I added some comments!
Ideally the API should take two parameters: addressbookId and contactsUID, that way we can also create commands/cron jobs that trigger the sync in the background (in a later stage maybe, but better plan ahead 😉 )

Good job!

call-me-matt · 2020-04-15T17:08:29Z

Thank you for your feedback! Reducing it to the basic function is fine for me (but later on, I think the background trigger would be a great feature for pictures). I did not implement any settings-page or background function.

Now thinking about naming/icons again, maybe the sync-image is not optimal, as it implies bi-directional dataflow.

Coming to your proposed structure, let me see if I understood it correctly:
You would prefer to rename the AvatarController to ApiController and in there implement the logic? So the ContactDetailsAvatar-Menu gets the image from window.location.href + '/social/avatar/', linked by the route ['name' => 'api#social', 'url' => '/{group}/{contact}/social/{type}/ ...]? Then, in the ApiController, I would have the social-Function that extracts and verifies the social profile from the contact, compiles the respective URL to the social network endpoint and downloads the picture (if type=avatar) or returns other contact information (if type is something else - later). That means that I have to convert my code from javascript to php, right? And by passing the information this way, I receive strings for group and contact, but am not sure how to access the information of these. Before I dig into that, could you indicate if I understood your suggestions correctly?

codecov · 2020-04-15T18:27:38Z

Codecov Report

Merging #1580 into master will decrease coverage by 31.97%.
The diff coverage is 25.33%.

@@              Coverage Diff              @@
##             master    #1580       +/-   ##
=============================================
- Coverage     67.16%   35.19%   -31.98%     
- Complexity       15      102       +87     
=============================================
  Files             5       14        +9     
  Lines            67      287      +220     
=============================================
+ Hits             45      101       +56     
- Misses           22      186      +164

Impacted Files	Coverage Δ	Complexity Δ
appinfo/routes.php	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
lib/AppInfo/Application.php	`0.00% <ø> (ø)`	`2.00 <0.00> (ø)`
lib/Controller/SocialApiController.php	`0.00% <0.00%> (ø)`	`4.00 <4.00> (?)`
lib/Service/Social/CompositeSocialProvider.php	`0.00% <0.00%> (ø)`	`10.00 <10.00> (?)`
lib/Service/Social/FacebookProvider.php	`0.00% <0.00%> (ø)`	`5.00 <5.00> (?)`
lib/Service/Social/InstagramProvider.php	`0.00% <0.00%> (ø)`	`10.00 <10.00> (?)`
lib/Service/Social/MastodonProvider.php	`0.00% <0.00%> (ø)`	`9.00 <9.00> (?)`
lib/Service/Social/TumblrProvider.php	`0.00% <0.00%> (ø)`	`6.00 <6.00> (?)`
lib/Service/Social/TwitterProvider.php	`0.00% <0.00%> (ø)`	`12.00 <12.00> (?)`
lib/Settings/AdminSettings.php	`0.00% <0.00%> (ø)`	`5.00 <5.00> (?)`
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e91667...09ef3ee. Read the comment docs.

Signed-off-by: matthias <matthias@butler> Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de> Signed-off-by: matthias <nextcloud@matthiasheinisch.de> Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de> Signed-off-by: call-me-matt <matthias@butler> Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de> Signed-off-by: call-me-matt <matthias@butler> Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

karlitschek · 2020-05-17T13:43:47Z

Regarding the legal question. I am 99% sure it is ok to download public pictures for private use. A browser does the same for the local cache. Regarding the last remaining 1%. IF this is illegal then it’s a problem for the person who clicks the button and not us providing the software. So it’s fine to merge IMHO

karlitschek · 2020-05-17T14:15:26Z

But disabled by default I think 😀

lib/Service/SocialApiService.php

Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

skjnldsv

Woooooow, I'm just soooo happy with your work!!
Sorry again I took that long to have the time to dive into this again!
It. Is. Awesome!!!

I added a few comments, but it works very nicely, I'm very happy about the code quality too!!
Very VERY well done @call-me-matt !!!

css/icons.scss

lib/Service/Social/CompositeSocialProvider.php

src/components/AdminSettings.vue

src/adminSettings.js

src/components/ContactDetails/ContactDetailsAvatar.vue

skjnldsv · 2020-07-09T15:01:21Z

src/components/ContactDetails/ContactDetailsAvatar.vue

+					// Notify user
+					OC.Notification.showTemporary(t('contacts', 'Avatar downloaded from social network'))


I guess the avatar being loaded is enough, no need for another feedback?

Suggested change

// Notify user

OC.Notification.showTemporary(t('contacts', 'Avatar downloaded from social network'))

I noticed that in some cases the picture is refreshed but unchanged (to the eye). So that is why I introduced this info popup. I think it happens when the picture is converted in the contact and not byte-identical to the downloaded one. In these cases it looks as if nothing happened to the user, even though the image has been refreshed.

AH, then we can get this in and if @jancborchardt disagree we'll remove later, this is no blocker ! 😉

skjnldsv · 2020-07-09T15:03:44Z

@ChristophWurst could you do a quick review on the way we do the SocialProvider? This was a talk we had together :)

Just a code review see if you spot anything in the php structure that might be optimized/changed!
Thanks for your help there btw!! 🤗

appinfo/routes.php

lib/Controller/SocialApiController.php

lib/Service/Social/CompositeSocialProvider.php

lib/Service/SocialApiService.php

Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

call-me-matt · 2020-07-09T23:06:03Z

A big thank you for taking your time and all your valuable comments! I started implementing them already (resolving the conversations here as I advance) and will push tomorrow.

Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

skjnldsv

Please also rebase to latest master
I merged our automated php formatting testing Could be nice to have this in as well :)

(Aside from formatting, nothing was done on the conflicting test file, so feel free to drop master changes for tests/unit/Controller/PageControllerTest.php)

skjnldsv · 2020-07-10T07:19:53Z

Please also rebase to latest master
I merged our automated php formatting testing Could be nice to have this in as well :)

(Aside from formatting, nothing was done on the conflicting test file, so feel free to drop master changes for tests/unit/Controller/PageControllerTest.php)

Before doing this, please squash all your 75 commits (it's too much 🙈) into one or two commits please 😁

skjnldsv · 2020-07-10T07:37:06Z

So, This was actually quite complicated, so I did it on another branch.
Please have a look @call-me-matt
It will be cleaner that way, the rebase was a nightmare xD
#1692

Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>

call-me-matt force-pushed the master branch 4 times, most recently from 32bd255 to 1d38907 Compare April 14, 2020 19:30

skjnldsv added 2. developing Work in progress enhancement New feature or request labels Apr 15, 2020